Tests #99
Tests #99
Conversation
Updated the docstring to clarify test purpose.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## add_authentication #99 +/- ##
=======================================================
+ Coverage 99.85% 100.00% +0.14%
=======================================================
Files 21 21
Lines 686 690 +4
=======================================================
+ Hits 685 690 +5
+ Misses 1 0 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds authentication support for Elasticsearch connections, enabling the application to work with authentication-enabled Elasticsearch instances. The changes implement Basic and Bearer authentication mechanisms and update the test infrastructure to support testing with authentication.
- Adds authentication header support to Elasticsearch requests via
get_elasticsearch_auth_header() - Updates docker-compose configuration to enable Elasticsearch security with default credentials
- Implements comprehensive test coverage for authentication scenarios
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_auth_integration.py | New integration tests verifying authentication requirements and credential validation |
| tests/unit/es_client/test_es_client.py | Unit tests for authentication header handling in search requests |
| tests/unit/conftest.py | Test configuration updated to set authentication environment variables |
| tests/helpers/unit_setup.py | Service startup configuration with authentication credentials |
| tests/helpers/init_elasticsearch.py | Updated to use configurable Elasticsearch URL from environment |
| src/search2_rpc/service.py | Modified to add authentication headers to index listing requests |
| docker-compose.yaml | Enabled Elasticsearch security with authentication credentials |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import os | ||
| # Set environment variables BEFORE any other imports | ||
| # This ensures the config module picks up the auth credentials | ||
| os.environ['ELASTICSEARCH_URL'] = 'http://localhost:9200' | ||
| os.environ['ELASTICSEARCH_AUTH_USERNAME'] = 'elastic' | ||
| os.environ['ELASTICSEARCH_AUTH_PASSWORD'] = 'changeme' | ||
|
|
||
| import pytest # noqa: E402 | ||
| from tests.helpers.unit_setup import ( # noqa: E402 |
There was a problem hiding this comment.
Setting environment variables directly in conftest.py creates hardcoded test configuration. Consider using pytest fixtures or a test configuration file to make these values configurable and avoid duplicating the same environment variable setup that also appears in unit_setup.py (lines 29-31).
| # Set environment variables BEFORE importing config module | ||
| # This ensures config picks up the auth credentials | ||
| os.environ['ELASTICSEARCH_URL'] = 'http://localhost:9200' | ||
| os.environ['ELASTICSEARCH_AUTH_USERNAME'] = 'elastic' | ||
| os.environ['ELASTICSEARCH_AUTH_PASSWORD'] = 'changeme' |
There was a problem hiding this comment.
This duplicates the exact same environment variable configuration found in tests/unit/conftest.py (lines 5-7). Centralizing this configuration in one location would improve maintainability and prevent potential inconsistencies.
No description provided.